Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix ECS fields in Elastic Log Driver, change index prefix #20522

Merged
merged 7 commits into from
Aug 12, 2020
Merged

Fix ECS fields in Elastic Log Driver, change index prefix #20522

merged 7 commits into from
Aug 12, 2020

Conversation

fearful-symmetry
Copy link
Contributor

@fearful-symmetry fearful-symmetry commented Aug 10, 2020 •
edited
Loading

What does this PR do?

  • Report the correct hostname and properly separated container names in ECS fields
  • Change the prefix to logs-docker so documents show up in the Kibana logs UI

Why is it important?

Although the ECS fields themselves are an issue, we also want Log Driver documents to be easily discoverable in Kibana with little additional configuration. The Logs UI will look for logs-* index patterns by default.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

  • Checkout PR locally
  • Run mage BuildAndInstall
  • Run the plugin using the examples provided in the docs
  • Make sure any host.* and container.* fields are correct, and that logs show up in the Kibana log UI
  • The end result should look something like this:
{
  "_index": ".ds-logs-docker-8.0.0-2020.08.10-000001",
  "_type": "_doc",
  "_id": "_p2a2nMBsYRBGP3GiK5-",
  "_version": 1,
  "_score": null,
  "_source": {
    "@timestamp": "2020-08-10T23:00:26.046Z",
    "container": {
      "labels": {},
      "id": "6abc8bcd301162fe156d0d560362501a9215c8e9e24e582247ad49dfa9ed1872",
      "name": "awesome_heisenberg",
      "image": {
        "name": "redis",
        "tag": "latest"
      }
    },
    "host": {
      "name": "buzzard.nest"
    },
    "ecs": {
      "version": "1.5.0"
    },
    "agent": {
      "type": "elastic-log-driver",
      "version": "8.0.0",
      "ephemeral_id": "617bcc47-3640-4866-8725-6f22546d53ba",
      "id": "a5308b40-344e-41ab-a985-212075a9762a",
      "name": "buzzard.nest"
    },
    "message": "1:M 10 Aug 2020 23:00:26.045 # WARNING overcommit_memory is set to 0! Background save may fail under low memory condition. To fix this issue add 'vm.overcommit_memory = 1' to /etc/sysctl.conf and then reboot or run the command 'sysctl vm.overcommit_memory=1' for this to take effect."
  },
  "fields": {
    "@timestamp": [
      "2020-08-10T23:00:26.046Z"
    ]
  },
  "sort": [
    1597100426046
  ]
}

@fearful-symmetry fearful-symmetry self-assigned this Aug 10, 2020
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Aug 10, 2020
@fearful-symmetry fearful-symmetry requested a review from a team August 10, 2020 19:44
@fearful-symmetry fearful-symmetry added the Team:Integrations Label for the Integrations team label Aug 10, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Aug 10, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Aug 10, 2020 •
edited by jenkins-beats-ci bot
Loading

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #20522 updated]

  • Start Time: 2020-08-12T16:34:50.562+0000

  • Duration: 29 min 56 sec

Test stats 🧪

Test Results
Failed 0
Passed 3
Skipped 0
Total 3

urso
urso reviewed Aug 10, 2020
View reviewed changes
x-pack/dockerlogbeat/main.go
hostname, err := os.Hostname()
if err != nil {
fatal("Error fetching hostname: %s", err)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of creating and passing the hostname around from here, would it make sense to create a beat.Info instead?

urso
urso reviewed Aug 10, 2020
View reviewed changes
x-pack/dockerlogbeat/docs/configuration.asciidoc Outdated
|`name`
|`testbeat`
| A custom value that will be inserted into the document as `agent.name`.
If not set, it will be the same value as `agent.type`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a little confusing for Beats users. Libbeat also provides a 'global' name setting, but defaults to the hostname if name is not configured. This is done, because name is used by users to identify a single instance (e.g. if multiple beats are run on a single host). Do we want to default to the hostname instead of agent.type as well?

@fearful-symmetry
Copy link
Contributor Author

Yah, @urso The use of name and such within libbeat is...really confusing and I wasn't totally sure how to handle it. beat.Info has a hostname field, but it's ignored if name is empty. This was the least awkward way I could think of to many sure host.hostname had an actual hostname, since it tends to get overwritten with agent.name if you're not careful. What should the default value for agent.Name be? The hostname? agent.type ?

@fearful-symmetry fearful-symmetry requested review from urso and a team August 11, 2020 16:13
@urso
Copy link

urso commented Aug 11, 2020

What should the default value for agent.Name be? The hostname? agent.type ?

The hostname.

According to our docs the 'name' setting defaults to hostname: https://www.elastic.co/guide/en/beats/filebeat/current/configuration-general-options.html#_name

NewBeat also initializes Name to hostname. The Name field will be overwritten by the beatConfig.Name, if beatConfig.Name is not empty.

@fearful-symmetry
Copy link
Contributor Author

So, right now it looks like we're "in line" with how other parts of libbeat work. We default to the hostname if the user doesn't set anything.

@fearful-symmetry
Copy link
Contributor Author

Alright, fixed the docs so hopefully it's a little more clear.

@fearful-symmetry fearful-symmetry merged commit a8f9cc8 into elastic:master Aug 12, 2020
@fearful-symmetry fearful-symmetry mentioned this pull request Aug 12, 2020
Merged
6 tasks
fearful-symmetry added a commit to fearful-symmetry/beats that referenced this pull request Aug 12, 2020
@fearful-symmetry
Fix ECS fields in Elastic Log Driver, change index prefix (elastic#20522
a10c6ba 
)

* change index names, clean up code

* update docs

* fix up metadata handling

* fix docs

* add changelog entry

(cherry picked from commit a8f9cc8)
fearful-symmetry added a commit that referenced this pull request Aug 17, 2020
@fearful-symmetry
Fix ECS fields in Elastic Log Driver, change index prefix (#20522) (#…
40cb94c 
…20577)

* change index names, clean up code

* update docs

* fix up metadata handling

* fix docs

* add changelog entry

(cherry picked from commit a8f9cc8)
melchiormoulin pushed a commit to melchiormoulin/beats that referenced this pull request Oct 14, 2020
@fearful-symmetry @melchiormoulin
Fix ECS fields in Elastic Log Driver, change index prefix (elastic#20522
5e898a6 
)

* change index names, clean up code

* update docs

* fix up metadata handling

* fix docs

* add changelog entry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@urso urso urso approved these changes

Assignees

@fearful-symmetry fearful-symmetry

Labels
enhancement Team:Integrations Label for the Integrations team v7.10.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fearful-symmetry @elasticmachine @urso